Skip to content

fix(terminal): scroll a phone drag by finger travel, not per touchmove - #963

Merged
devswha merged 6 commits into
mainfrom
fix/touch-scroll-by-finger-travel
Oct 13, 2026
Merged

devswha merged 6 commits into
mainfrom
fix/touch-scroll-by-finger-travel

Conversation

@devswha

@devswha devswha commented Oct 13, 2026 •

Copy link
Copy Markdown
Owner

Problem

On a phone, a slow drag scrolls herdr's history steadily, but a quick swipe over the same distance moves only a few notches. The text does not follow the finger, and flicking through a long agent transcript takes many repeated drags.

Cause

onTouchMove in PaneTerminal.tsx dispatched one synthetic pixel-mode WheelEvent per touchmove. With mouse reporting on (herdr attach), xterm 5.5 sends exactly one wheel report per wheel event: getLinesScrolled is used only as a zero check, and its _wheelPartialScroll %= 1 drops every whole row beyond the first. The wheel-speed replay only covers trusted events, so touch got no compensation. A 40-60 px touchmove (a flick) crossed 2-3 rows but sent one report; a slow drag sent about one per row.

Fix

  • New pure helper src/lib/wheelSteps.ts: wheelSteps(pixels, stepPixels) returns the whole signed steps and the pixels left over. HERDR_MOUSE_SCROLL_LINES = 3 is herdr's default ui.mouse_scroll_lines.
  • The touch handler gathers the gesture's travel (touchPixels, reset on touchstart), divides it by cell height x 3, and dispatches that many DOM_DELTA_LINE wheels of deltaY +-1 (each makes xterm send exactly one report), keeping the rest for the next move. One report then moves herdr about as far as the finger moved.
  • The selecting drag's wheel (dragWheel) uses the same helper and the same cellHeight() so the two conversions cannot drift.
  • src/AGENTS.md geometry row updated.

No momentum in this PR: a flick still stops where the finger lifts (onTouchEnd sends at most one leftover step, see the merge-session review below). render-scroll-2 also proposed a decaying momentum phase; that is deferred to a follow-up and this PR closes only the per-touchmove report loss.

Tests

  • scripts/terminal-pinch-regression.ts (browser, runs in ui-contexts-regression): at the 13 px baseline it drags 300 px once in 2 touchmoves and once in 30, and asserts both send the same wheel count (about 300 / (3 x cell height)) and that every wheel is DOM_DELTA_LINE. Verified to fail on main's PaneTerminal.tsx ("a 2-move swipe sends line-mode wheels only"). The finger left after a partial pinch release now moves three wheel steps of the 20 px cell (it moved 60 px, under one step, so that negative check could not fail).
  • src/lib/wheelSteps.test.ts (new): five 60 px moves send the same report count as thirty 10 px moves (and 300 x 1 px equals one 300 px move); sign and remainder carry; a reversal pays back gathered travel first; a zero/NaN cell height makes no steps.
  • bun run typecheck: pass. bun run test:unit: 0 fail.
  • bun run build, then bun run check run bun scripts/ui-contexts-regression.ts: pass, including checkTerminalPinch (earlier commit); after the test update, bun run check run bun scripts/terminal-pinch-regression.ts: pass. Its one-finger drag went from 60 px to 150 px: at a 20 px font, 60 px is less than one three-row step, so it would no longer send a wheel. That script only asserts that synthetic wheel events are dispatched; it does not check how far herdr scrolls.
  • scripts/windows-lens-browser-qa.ts (manual QA, not a CI lane) is the only script that drags a finger into real herdr history. It failed twice at an earlier step ("the command's output reaches the mirrored terminal", in the mirror-only part), before the touch-scroll step ran. So the herdr-side scroll distance was not verified end to end here.

Notes

🤖 Generated with Claude Code

Merge-session review

Fixed (b462b57):

  • Short drags no longer get lost. Before this, a one-finger flick shorter than one step (3 rows, so 51 px at the default 13 px font) sent no wheel at all, and every longer drag dropped its last partial step. Now, on touchend of a one-finger drag (not a pinch, not an adopted grid, not a touchcancel), one more line-mode wheel is sent if at least half a step is left (finalStep in src/lib/wheelSteps.ts). Unit cases cover finalStep. scripts/terminal-pinch-regression.ts now checks that a 0.7-step flick sends exactly one wheel and a 0.3-step flick sends none. On the previous head this case failed: "a flick of 0.7 step (40 px) sends one wheel". The changelog fragment and the src/AGENTS.md row now describe the touchend step.

Deferred:

  • Momentum/inertia scrolling after a fling (already listed above). This is still out of scope.

Checks: bun run check fast passes, and CI=1 bun run check run --build bun scripts/terminal-pinch-regression.ts passes.

devswha and others added 2 commits October 13, 2026 21:43
xterm sends herdr one wheel report per wheel event whatever its delta, and the
touch handler dispatched one pixel wheel per touchmove, so a quick swipe sent a
few reports while a slow drag over the same distance sent one per row.

Gather the gesture's travel and send one line-mode wheel per cell height times
herdr's default mouse_scroll_lines (3), keeping the rest for the next move. The
pixel-to-step arithmetic is shared with the selecting drag's wheel.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 13, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 39 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 4 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Repository: devswha/herdr-web-ui/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 331d08bd-2c64-49bc-8d86-48cad9e7972d
📥 Commits

Reviewing files that changed from the base of the PR and between b462b57 and 1304eae.

📒 Files selected for processing (6)
  • changelog.d/touch-scroll-by-finger-travel.md
  • scripts/terminal-pinch-regression.ts
  • src/AGENTS.md
  • src/components/PaneTerminal.tsx
  • src/lib/wheelSteps.test.ts
  • src/lib/wheelSteps.ts

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: devswha/herdr-web-ui/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 83166df6-c9f6-457d-a1ea-4ffd1fab16b7
📥 Commits

Reviewing files that changed from the base of the PR and between 5a4bfb3 and b462b57.

📒 Files selected for processing (6)
  • changelog.d/touch-scroll-by-finger-travel.md
  • scripts/terminal-pinch-regression.ts
  • src/AGENTS.md
  • src/components/PaneTerminal.tsx
  • src/lib/wheelSteps.test.ts
  • src/lib/wheelSteps.ts

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 2 remain after this review.

📝 Summary

Summary by CodeRabbit

  • Improvements
    • Phone terminal scrolling now follows finger travel, producing one scroll step for every three rows moved.
    • A partial movement of at least half a step scrolls once when you lift your finger.

Terminal touch scrolling now accumulates finger travel and emits line-mode wheel reports in three-row steps. At touch end, a remaining partial step emits one report when it reaches half a step. Shared helpers also handle pixel-based wheel input.

Changes: Terminal touch scrolling

Layer / File(s) Summary
Scroll-step conversion
src/lib/wheelSteps.ts, src/lib/wheelSteps.test.ts
Adds helpers to convert pixel travel into signed whole steps and evaluate the remaining travel at touch end. Tests cover distance, direction, remainders, and invalid values.
Terminal integration and regression coverage
src/components/PaneTerminal.tsx, scripts/terminal-pinch-regression.ts, src/AGENTS.md, changelog.d/touch-scroll-by-finger-travel.md
PaneTerminal uses the helpers to accumulate touch travel and dispatch line-mode wheel reports. Regression checks cover swipe travel and pinch behavior. Documentation and the changelog describe the updated scrolling behavior.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to b462b

Phone drag scrolling now follows finger travel instead of the number of touchmove events. No concrete merge-blocking risk was found. On PCs with a non-default scroll-lines setting, the scroll distance may differ from finger travel, which is a known and documented limitation.

Pre-merge checks · Passed 5
Passed Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage Passed Docstring coverage is 80.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 4 files. (2 skipped: 2 u…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Title check Passed The title clearly and concisely describes the primary change: phone terminal scrolling now follows finger travel instead of generating one scroll per touchmove.
Description check Passed The description explains the problem, cause, fix, scope, limitations, and validation results. It uses Problem, Fix, and Tests sections instead of the template's Change and Validation headings, but it …
✨ Finishing Touches 💡 1
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
🛠️ Fix failing CI checks 💡
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

devswha and others added 4 commits October 13, 2026 21:52
…inger move by cell height

The pinch check now drags 300 px once in 2 touchmoves and once in 30 and
asserts both send the same count of line-mode wheels, about one per three
rows. On the old per-touchmove dispatch this fails. The finger left after a
partial pinch release now moves three wheel steps at the 20 px cell, so a
broken pinch-stroke guard would send wheels there.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…r lifts

Gathering finger travel into one wheel report per three rows dropped any
flick shorter than a step: the remainder reset at each touchstart and
touchend never sent it, so 30-40 px nudges at the default font scrolled
nothing. On touchend of a one-finger drag (not a pinch, not an adopted
grid, not a cancel), send one more report when at least half a step is
left (finalStep in lib/wheelSteps.ts). Unit cases for finalStep and a
browser case in terminal-pinch-regression: a 0.7-step flick sends one
wheel, a 0.3-step flick sends none.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The pinch's touchStart moved finger 1 back 90 px, which Chrome reports as a
one-finger touchmove first; with the drag's leftover that crossed a wheel step
at CI's cell height.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@devswha
devswha merged commit 4e105fa into main Oct 13, 2026
9 checks passed
@devswha
devswha deleted the fix/touch-scroll-by-finger-travel branch October 13, 2026 15:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant